Skip to content

Fixed #33 - Issue - Update the GuzzleHTTP package#34

Closed
pratikrm wants to merge 2 commits intomasterfrom
fixed-33
Closed

Fixed #33 - Issue - Update the GuzzleHTTP package#34
pratikrm wants to merge 2 commits intomasterfrom
fixed-33

Conversation

@pratikrm
Copy link
Copy Markdown
Contributor

Closes #33

@pratikrm pratikrm requested review from surajv and uzair-b November 14, 2023 15:53
@pratikrm pratikrm self-assigned this Nov 14, 2023
@pratikrm pratikrm requested review from balazspiller and removed request for uzair-b November 14, 2023 17:41
Copy link
Copy Markdown
Collaborator

@balazspiller balazspiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but I remember having some issue when I tried updating Guzzle the last time, something like a PHP compatibility issue. I don't remember exactly, and anyway this PR seems to work fine, so I'm approving it.

@pratikrm
Copy link
Copy Markdown
Contributor Author

I am afraid about the 2M+ changes, it increased the zip size by 6 MB

@balazspiller
Copy link
Copy Markdown
Collaborator

Now that you mention it, I remember we had the same issue even with the current version, and we solved it by deleting most of the unneeded code from the Google API library.

But that is probably not a great solution, because if another plugin uses the same Google API, and it's ours that gets loaded, then the missing code could cause problems on the site.

@pratikrm
Copy link
Copy Markdown
Contributor Author

@balazsrm, better if we use JavaScript or Node Library then:
https://github.com/google/google-api-javascript-client
https://github.com/googleapis/google-api-nodejs-client
?

@balazspiller
Copy link
Copy Markdown
Collaborator

The Node library is not really an option, because that can’t be run on PHP-based WP sites. The JS library might work but it would probably require extensive changes in the plugin. I’ll check what can be done.

@balazspiller
Copy link
Copy Markdown
Collaborator

New PR: #41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue - Update the GuzzleHTTP package

2 participants